-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Make ollama models info transport work like lmstudio #7679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and this PR successfully fixes the Ollama max context window display issue. The implementation follows the same pattern as LM Studio, which provides good consistency.
✅ Positive Aspects
- Correctly addresses the root cause where
infowas always undefined - Good consistency with LM Studio's implementation pattern
- Comprehensive test coverage added for the new functionality
- Type changes are properly propagated through the codebase
💭 Minor Suggestions for Improvement
-
Cache flushing in requestOllamaModels handler (webviewMessageHandler.ts line ~662)
- The cache flush ensures fresh models, which is good for accuracy
- However, if this handler is called frequently (e.g., on every settings page load), it might impact performance
- Consider whether the cache flush is always necessary, or if it could be conditional
-
Error message clarity (useOllamaModels.ts line ~16)
- The timeout error message could be more helpful
- Consider: "Ollama models request timed out. Please check if Ollama is running and accessible at the configured URL."
-
Conditional fetching logic (useOllamaModels.ts line ~39)
- The hook only fetches models when
modelIdis provided - This differs from the LM Studio implementation
- While this seems intentional, a comment explaining the reasoning would help future maintainers
- The hook only fetches models when
Overall Assessment
The implementation is solid and fixes the reported issue effectively. The code quality is good and follows existing patterns. The minor suggestions above are optional improvements that don't block the PR.
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @ItsOnlyBinary!
Related GitHub Issue
Closes: #7674
Roo Code Task Context (Optional)
Description
The task view while chatting was not showing ollama max context length.
I looked how LM Studio had been changed to retrieve its model info within the ui.
I used that as a template to change how ollama worked so the info is now shown.
Test Procedure
Start chatting with ollama
In Task Header you will see tokens usage as: used/8k
On current version it will show: used/1
Pre-Submission Checklist
Screenshots / Videos
before:

after:

Documentation Updates
Additional Notes
Get in Touch
Important
Update Ollama model handling to fetch and display detailed model information, aligning with LM Studio's approach.
webviewMessageHandlerto fetch Ollama models with full details instead of just IDs.requestOllamaModelsto flush cache and fetch fresh models.Ollama.tsxto handle and display detailed Ollama models.useOllamaModelshook to fetch and manage Ollama models.useSelectedModelto include Ollama models in selection logic.ollamaModelstype inExtensionMessage.tsfromstring[]toModelRecord.This description was created by
for 68df30d. You can customize this summary. It will automatically update as commits are pushed.